Skip to content

fix: prevent write_to_file from executing with missing content parameter#12080

Draft
roomote-v0[bot] wants to merge 2 commits intomainfrom
fix/write-to-file-missing-content-12079
Draft

fix: prevent write_to_file from executing with missing content parameter#12080
roomote-v0[bot] wants to merge 2 commits intomainfrom
fix/write-to-file-missing-content-12079

Conversation

@roomote-v0
Copy link
Copy Markdown
Contributor

@roomote-v0 roomote-v0 bot commented Apr 8, 2026 •
edited
Loading

Related GitHub Issue

Closes: #12079

Description

This PR attempts to address Issue #12079. Feedback and guidance are welcome.

Root Cause: When finalizeStreamingToolCall returns null (because the LLM sent malformed or incomplete arguments), the existing partial ToolUse block gets promoted to non-partial while retaining stale nativeArgs from partial parsing. The partial parser for write_to_file used OR logic (path || content), allowing nativeArgs to be set with only path present and content: undefined. This bypassed the safety check in presentAssistantMessage that guards against missing nativeArgs, causing WriteToFileTool.execute to receive params with content: undefined.

Fixes applied:

  1. Task.ts (two locations): Clear nativeArgs when promoting failed finalization blocks to complete. This ensures the safety check in presentAssistantMessage (!block.nativeArgs) properly catches these as invalid tool calls and returns a structured error to the LLM.

  2. NativeToolCallParser.ts: Tighten partial write_to_file nativeArgs to require both path AND content (changed || to &&). This is safe because handlePartial reads from block.params, not nativeArgs.

Test Procedure

  • Added 7 new tests in NativeToolCallParser.spec.ts covering:
    • Partial streaming with only path (no content) - verifies nativeArgs is not set
    • Partial streaming with both path and content - verifies nativeArgs is set
    • Complete parseToolCall with missing content - verifies returns null
    • Complete parseToolCall with both params - verifies correct parsing
    • Finalization with missing content - verifies returns null
    • Finalization with both params - verifies correct finalization
  • All 18 NativeToolCallParser tests pass
  • All 24 existing writeToFileTool tests pass
  • Full lint and type-check pass

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue.
  • Scope: My changes are focused on the linked issue.
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes.
  • Documentation Impact: No documentation updates are required.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

  • No documentation updates are required.

Interactively review PR in Roo Code Cloud

@roomote-v0 roomote-v0 bot mentioned this pull request Apr 8, 2026
Open
@roomote
fix: prevent write_to_file from executing with missing content parameter
4f0c595 
When finalizeStreamingToolCall returns null (malformed/incomplete args from
LLM), the existing partial ToolUse block was promoted to non-partial while
retaining stale nativeArgs from partial parsing. The partial parser for
write_to_file used OR logic (path || content), allowing nativeArgs to be
set with only path present. This bypassed the safety check in
presentAssistantMessage that guards against missing nativeArgs.

Two fixes applied:
1. Clear nativeArgs when promoting failed finalization blocks to complete
   in Task.ts (both finalization paths). This ensures the safety check
   in presentAssistantMessage properly catches invalid tool calls.
2. Tighten partial write_to_file nativeArgs in NativeToolCallParser to
   require both path AND content (handlePartial reads from params, not
   nativeArgs, so this is safe).

Closes #12079
@roomote-v0 roomote-v0 bot force-pushed the fix/write-to-file-missing-content-12079 branch from cb70c75 to 4f0c595 Compare April 10, 2026 11:25
@roomote-v0 roomote-v0 bot changed the title fix: allow partial args through parseToolCall finalization to enable proper retry behavior fix: prevent write_to_file from executing with missing content parameter Apr 10, 2026
@rockey5520
Copy link
Copy Markdown

rockey5520 commented Apr 10, 2026

@roomote this step is failing https://github.com/RooCodeInc/Roo-Code/actions/runs/24240632367/job/70774058524?pr=12080 can you check

@roomote-v0
Copy link
Copy Markdown
Contributor Author

roomote-v0 bot commented Apr 10, 2026 •
edited
Loading

Fixaroo Clock   See task

The failing preview check is not caused by this PR. It's the "Preview roocode.com" Vercel deployment workflow, which fails because packages/types compiled .js files can't be resolved during the Next.js build of apps/web-roo-code. This is a pre-existing infrastructure issue -- the same workflow has been failing on many other branches recently (renovate branches, changeset-release/main, etc.).

All code-related CI checks for this PR pass:

  • compile: pass
  • platform-unit-test (ubuntu + windows): pass
  • knip: pass
  • CodeQL: pass
  • check-translations: pass
  • check-secrets: pass

No code changes needed here.

@rockey5520
Copy link
Copy Markdown

rockey5520 commented Apr 10, 2026

@roomote still the status says "some checks were not successful"

@roomote-v0
Copy link
Copy Markdown
Contributor Author

roomote-v0 bot commented Apr 10, 2026 •
edited
Loading

Fixaroo Clock   See task

I investigated the failing check. The only failure is the preview job (Vercel website preview deployment), which fails with a Turbopack build error: Module not found: Can't resolve './api.js' in packages/types/src/index.ts. This is unrelated to this PR's changes -- this PR only modifies files in src/core/ and the preview workflow is scoped to apps/web-roo-code/** changes.

All actual code quality checks pass:

  • compile: pass
  • platform-unit-test (ubuntu-latest): pass
  • platform-unit-test (windows-latest): pass
  • CodeQL: pass
  • knip: pass
  • check-translations: pass

No code changes are needed in this PR to address the preview failure. It appears to be a pre-existing website build issue.

@rockey5520
Copy link
Copy Markdown

rockey5520 commented Apr 10, 2026

@roomote looks like this MR wont be considered to review till all the checks are passed per what i see in reviewer status

@roomote-v0
Copy link
Copy Markdown
Contributor Author

roomote-v0 bot commented Apr 10, 2026 •
edited
Loading

Fixaroo Clock   See task

The only failing check was preview (Vercel website preview deployment), which was triggered spuriously due to a force-push path filter evaluation issue in GitHub Actions. This PR doesn't change any apps/web-roo-code/** files, so the preview workflow should not have run. Pushed an empty commit to re-trigger CI with a clean (non-force) push so GitHub can properly evaluate path filters. All tests (18/18), lint, and type-checks pass locally.

View commit | Revert commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrubens mrubens Awaiting requested review from mrubens mrubens will be requested when the pull request is marked ready for review mrubens is a code owner

@cte cte Awaiting requested review from cte cte will be requested when the pull request is marked ready for review cte is a code owner

@jr jr Awaiting requested review from jr jr will be requested when the pull request is marked ready for review jr is a code owner

@hannesrudolph hannesrudolph Awaiting requested review from hannesrudolph hannesrudolph will be requested when the pull request is marked ready for review hannesrudolph is a code owner

@daniel-lxs daniel-lxs Awaiting requested review from daniel-lxs daniel-lxs will be requested when the pull request is marked ready for review daniel-lxs is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Intermittent failure: write_to_file called without required content parameter

2 participants

@rockey5520 @roomote